Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lightning-macros crate and drop bdk_macros dependency #3330

Merged
merged 2 commits into from
Oct 17, 2024

Conversation

tnull
Copy link
Contributor

@tnull tnull commented Sep 21, 2024

Previously, we used the bdk_macros dependency for some simple proc macros in lightning-transaction-sync. However, post-1.0 BDK doesn't further maintain this crate and will at some point probably yank it together with the old bdk crate that was split up.

Here, we create a new crate for utility proc macros and steal add what we currently use (slightly modified for the latest syn version's API though). In the future we may want to expand this crate, e.g., for some further changes to the maybe_async macros in the context of an async KVStore implementation.

dunxen
dunxen previously approved these changes Sep 22, 2024
Copy link
Contributor

@dunxen dunxen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks the same to me as bdk_macros besides the wasm flags removed 👍

@tnull
Copy link
Contributor Author

tnull commented Oct 8, 2024

Now pushed a fixup that removes the async trait functionality. While I think there is no real harm in having the original code around, happy to go this way, too. Let me know if I should squash or revert.

@TheBlueMatt
Copy link
Collaborator

Feel free to squash, then LGTM.

@tnull
Copy link
Contributor Author

tnull commented Oct 17, 2024

Feel free to squash, then LGTM.

Squashed fixup without further changes.

@TheBlueMatt
Copy link
Collaborator

Oops, the async-trait removal got squashed into the wrong commit :/

Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Total diff looks fine, even if the fixup was squashed into the wrong commit.

Previously, we used the `bdk_macros` dependency for some simple proc
macros in `lightning-transaction-sync`. However, post-1.0 BDK doesn't
further maintain this crate and will at some point probably yank it
together with the old `bdk` crate that was split up.

Here, we create a new crate for utility proc macros and ~~steal~~ add
what we currently use (slightly modified for the latest `syn` version's
API though). In the future we may want to expand this crate, e.g., for
some `maybe_async` macros in the context of an `async KVStore`
implementation.
@tnull
Copy link
Contributor Author

tnull commented Oct 17, 2024

Oops, the async-trait removal got squashed into the wrong commit :/

Grr🤦‍♂️

Fixed.

Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gonna land this, only really removed code since @dunxen last looked and its pretty straightforward.

@TheBlueMatt TheBlueMatt merged commit 81923c7 into lightningdevkit:main Oct 17, 2024
17 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants